Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ci] Script for building M1 wheels #19925

Merged
merged 4 commits into from
Nov 1, 2021
Merged

[ci] Script for building M1 wheels #19925

merged 4 commits into from
Nov 1, 2021

Conversation

wuisawesome
Copy link
Contributor

@wuisawesome wuisawesome commented Oct 31, 2021

Why are these changes needed?

This PR includes a script for building wheels for Macs with M1 processors. It roughly follows the pattern of the other scripts with a few differences.

  • Manually installs nvm
  • Uses miniforge conda to install python/pip instead of python foundation .pkgs
  • Doesn't pin numpy (we probably shouldn't be pinning it in the other scripts either...)
  • Commit detection falls back to git instead of erroring

All of these changes were made so that the script works on a laptop, which comes with a subset of the dependencies that the x86 buildkite image comes with.

Related issue number

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

wuisawesome and others added 2 commits October 31, 2021 13:02
This PR includes a script for building wheels for Macs with M1 processors. It roughly follows the pattern of the other scripts with a few differences. 

* Manually installs `nvm`
* Uses miniforge conda to install python/pip instead of python foundation .pkgs 
* Doesn't pin numpy (we probably shouldn't be pinning it in the other scripts either...)
* Commit detection falls back to `git` instead of erroring

All of these changes were made so that the script works on a laptop, which comes with a subset of the dependencies that the x86 buildkite image comes with.
@wuisawesome
Copy link
Contributor Author

@gjoliver this script works on my laptop, it would be great to make sure that it works on at least one other machine (i.e. yours).

Copy link
Member

@gjoliver gjoliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good! thanks.

@@ -84,7 +79,6 @@ for ((i=0; i<${#PY_VERSIONS[@]}; ++i)); do
TRAVIS_COMMIT=$(git rev-parse HEAD)
fi

echo $TRAVIS_COMMIT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not keep this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh i mean i guess we could, it was just an accidental debugging print statement i left in.

(note we are already printing echo "TRAVIS_COMMIT variable detected. ray.__commit__ will be set to $TRAVIS_COMMIT")

@gjoliver
Copy link
Member

gjoliver commented Nov 1, 2021

also, maybe we should rename the script to build-wheel-macos-arm64.sh
but that is an extremely minor comment.

Copy link
Contributor

@simon-mo simon-mo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@simon-mo simon-mo added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Nov 1, 2021
@simon-mo
Copy link
Contributor

simon-mo commented Nov 1, 2021

leaving up to @wuisawesome to merge

@wuisawesome wuisawesome merged commit 80fb3f1 into master Nov 1, 2021
@wuisawesome wuisawesome deleted the wuisawesome-patch-5 branch November 1, 2021 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants